Added support for multipart form data in OpenAPI-to-JAX-RS generator#391
Added support for multipart form data in OpenAPI-to-JAX-RS generator#391EricWittmann merged 4 commits intoApicurio:mainfrom
Conversation
| // Try to extract method name from path segment | ||
| if (currentPath != null && currentPath.length() > 0) { | ||
| String[] pathSegments = currentPath.split("/"); | ||
| if (pathSegments.length > 0) { | ||
| String lastSegment = pathSegments[pathSegments.length - 1]; | ||
| if (lastSegment != null && lastSegment.trim().length() > 0) { | ||
| // Remove path parameters (e.g., {id}) and sanitize | ||
| String sanitized = lastSegment.replaceAll("\\{[^}]*\\}", "").replaceAll("\\W", ""); | ||
| if (sanitized.length() > 0) { | ||
| return sanitized; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
@EricWittmann is this okay here? I needed to add this, because the generated method was unexpectedly named "generatedMethod1". I also edited the chines character test accordingly.
There was a problem hiding this comment.
It's fine to add it here, but maybe we need to check for conflicts with names generated the other way?
There was a problem hiding this comment.
Maybe, I missed something, but all tests still run. Is there a specific test which covers the name generated the other way?
There was a problem hiding this comment.
Most of the tests cover the name generated from the operationId - but if there is an operationId with the same value as what is produced by this new code path, then we would have a conflict. I guess we would need a new test for that - basically an OpenAPI with an Operation that has an operationId that is the same as another operation without an operationId that will get resolved using this new approach.
…uestBodyProcessor` for clarity
EricWittmann
left a comment
There was a problem hiding this comment.
I love this! You've done a lot of work here. I would need to dig a lot more into this space to really give this a proper review, but it looks pretty good overall. I have some comments/requests for changes. In addition to those, here is some high level feedback.
The biggest issue is that it's fully coupled to OpenAPI 3.1. Unless we are auto-converting the spec to OpenAPI 3.1 before running this code, we'll need to support both 3.0 and 3.1. Are we auto-converting to 3.1?! We might be...
Maybe some error test cases would be useful:
- Invalid schema types
- Missing required fields
- Null schemas
- Non-multipart content types
- Mixed content types (multipart + json)
core/src/test/java/io/apicurio/hub/api/codegen/OpenApi2JaxRsTest.java
Outdated
Show resolved
Hide resolved
core/src/main/java/io/apicurio/hub/api/codegen/jaxrs/MultipartFormDataRequestBodyProcessor.java
Outdated
Show resolved
Hide resolved
core/src/main/java/io/apicurio/hub/api/codegen/jaxrs/MultipartFormDataRequestBodyProcessor.java
Show resolved
Hide resolved
core/src/main/java/io/apicurio/hub/api/codegen/OpenApi2JaxRs.java
Outdated
Show resolved
Hide resolved
core/src/main/java/io/apicurio/hub/api/codegen/jaxrs/MultipartFormDataRequestBodyProcessor.java
Outdated
Show resolved
Hide resolved
core/src/main/java/io/apicurio/hub/api/codegen/jaxrs/OpenApi2CodegenVisitor.java
Outdated
Show resolved
Hide resolved
| // Try to extract method name from path segment | ||
| if (currentPath != null && currentPath.length() > 0) { | ||
| String[] pathSegments = currentPath.split("/"); | ||
| if (pathSegments.length > 0) { | ||
| String lastSegment = pathSegments[pathSegments.length - 1]; | ||
| if (lastSegment != null && lastSegment.trim().length() > 0) { | ||
| // Remove path parameters (e.g., {id}) and sanitize | ||
| String sanitized = lastSegment.replaceAll("\\{[^}]*\\}", "").replaceAll("\\W", ""); | ||
| if (sanitized.length() > 0) { | ||
| return sanitized; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
It's fine to add it here, but maybe we need to check for conflicts with names generated the other way?
|
Hey @lpieprzyk - I was out on vacation last week. What's the status of this PR? Ready to go? All requested changes applied? Or not yet? |
I still have a small bug I’m working on, but I hope to be done by the end of the week. |
|
Cool no worries. I'm not pushing you - I just wanted to make sure you weren't waiting on me. :) |
| /** | ||
| * Checks for duplicate method names within the current interface and throws a | ||
| * {@link IllegalStateException} if a conflict is detected. This can occur when multiple | ||
| * operations resolve to the same method name, typically due to missing or identical | ||
| * {@code operationId} values in the OpenAPI specification. | ||
| * | ||
| * @param method the new method to check against existing methods in the current interface | ||
| * @throws IllegalStateException if a method with the same name already exists in the interface | ||
| */ | ||
| private void checkForDuplicateMethodNames(CodegenJavaMethod method) { | ||
| String newMethodName = method.getName(); | ||
| for (CodegenJavaMethod existingMethod : this._currentInterface.getMethods()) { | ||
| if (existingMethod.getName().equals(newMethodName)) { | ||
| throw new IllegalStateException( | ||
| "Duplicate method name '" + newMethodName + "' detected in interface '" | ||
| + this._currentInterface.getName() + "'. " | ||
| + "Operation at path '" + currentPath + "' (HTTP " + method.getMethod() | ||
| + ") resolves to the same method name as an existing operation. " | ||
| + "Consider adding a unique 'operationId' to one of the conflicting operations." | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
@EricWittmann Is this okay with you? In my opinion it is a configuration error in the spec file, if there are two endpoints with the same name. Or should I generate the endpoints with a counter suffix?
|
@EricWittmann I added a testcase for a 3.0.0 Spec. And it seems like we are auto-converting :D |
|
Test cases look good to me! 👍 Let me know if/when you think it's ready to merge and I'll take a final look before merging. |
|
I think all requested changes (if I didn´t miss anything) are done. |
|
Merged! 🎉 |
|
Hi @EricWittmann, could you generate a new release? |
|
In progress. |
No description provided.